[NAE-2405] QRService dependency rework#430
Conversation
- Replace `qrgen` library with `zxing` for QR code generation. - Add `QrImageType` enum for supported image format handling. - Refactor `QrService` to improve error handling and scaling logic. - Introduce helper methods for `BitMatrix` creation and image format resolution. - Update dependencies and clean up unused repository configuration in `pom.xml`.
- Add error correction level validation for embedded logo support. - Implement scaling and compositing methods for QR codes with logos. - Enhance logging for QR generation, including dimensions and errors. - Refactor `QrService` by adding helper methods for scaling and file writing. - Update `QrCode` to include defaults for logo ratio, background padding, and arc.
- Refactor logging to maintain consistent formatting across methods. - Adjust log messages for QR generation, scaling, and compositing methods. - Simplify multi-line logs into single-line statements for clarity.
WalkthroughReplaces qrgen with ZXing for QR generation, adds a QrImageType enum, introduces QR image/logo rendering defaults and higher error-correction in QrCode, and refactors QrService to encode BitMatrix via ZXing, write images via MatrixToImageWriter/ImageIO, and composite optional logos. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant QrService
participant ZXing as QRCodeWriter/MatrixToImageWriter
participant ImageIO
participant FileSystem
Client->>QrService: generate QR (QrCode)
QrService->>ZXing: encode text -> BitMatrix (hints: charset, ECC, margin)
alt no logo
ZXing->>QrService: image bytes
QrService->>Client: return InputStream / saved file
else with logo
QrService->>ImageIO: read logo InputStream -> BufferedImage
QrService->>QrService: scale logo, draw white rounded-rect background, composite centered
QrService->>ImageIO: write final BufferedImage (format from QrImageType)
ImageIO->>FileSystem: persist file (if saving)
QrService->>Client: return InputStream / saved file
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.java (1)
5-5:⚠️ Potential issue | 🟡 MinorRemove stale import from replaced library.
The import
net.glxn.qrgen.core.image.ImageTypereferences the old qrgen library being replaced by ZXing. This import appears unused now thatimageTypeusesQrImageTypeinstead.Proposed fix
-import net.glxn.qrgen.core.image.ImageType;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.java` at line 5, Remove the stale import of net.glxn.qrgen.core.image.ImageType from the QrCode class: locate the import statement referencing ImageType in QrCode.java and delete it so the class only uses the new ZXing-backed QrImageType; ensure no other references to ImageType remain and compile to verify the unused import is gone.application-engine/pom.xml (1)
416-421:⚠️ Potential issue | 🟠 MajorRemove the old QRGen dependency — it's no longer used.
The
net.glxn.qrgen:core:2.0dependency remains in the POM, but the code has been migrated to use ZXing (com.google.zxing:coreandcom.google.zxing:javase). This creates unnecessary classpath bloat and potential conflicts.Additionally,
QrCode.javastill importsnet.glxn.qrgen.core.image.ImageType, which will cause a compile error if this dependency is removed — that import should be cleaned up as well.Proposed fix: Remove the stale dependency
- <!-- QRGen --> - <dependency> - <groupId>net.glxn.qrgen</groupId> - <artifactId>core</artifactId> - <version>2.0</version> - </dependency>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application-engine/pom.xml` around lines 416 - 421, Remove the stale net.glxn.qrgen:core dependency block from the POM (the <dependency> element for groupId net.glxn.qrgen and artifactId core) and in the QrCode.java class remove the unused import net.glxn.qrgen.core.image.ImageType and any references to ImageType; replace those references with the appropriate ZXing/Java types used elsewhere (e.g., java.awt.image.BufferedImage or ZXing's matrix-to-image utilities) so the class compiles after the dependency is removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.java`:
- Line 29: The default offColor field in QrCode (offColor) is set to 0xFFFF99FF
(magenta) which makes QR backgrounds nonstandard; change the default to white
(0xFFFFFFFF) by updating the offColor declaration in class QrCode so
MatrixToImageConfig (used by QrService methods that construct
MatrixToImageConfig) will produce white backgrounds by default; ensure only the
offColor variable is modified and leave onColor and any callers intact.
In
`@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrService.java`:
- Around line 227-231: validateLogoErrorCorrection warns but every
generateWithLogo call will always log because QrCode.java defaults to
ErrorCorrectionLevel.L; instead ensure the error correction is elevated when a
logo is used: in the code path that handles logo generation (e.g.,
generateWithLogo in QrService or the QrCode builder), detect when a logo is
present and if code.getErrorCorrectionLevel() is lower than
ErrorCorrectionLevel.H, set or override the errorCorrectionLevel to
ErrorCorrectionLevel.H (or fail fast if you prefer stricter behavior) before
calling validateLogoErrorCorrection so that the default no longer triggers
spurious warnings; update Javadoc on QrCode/errorCorrectionLevel to note this
behavior.
- Around line 94-106: The catch in generateFile swallows IOExceptions and
returns outputPath.toFile() even when writing failed; either remove the
try-catch around MatrixToImageWriter.writeToStream (letting the declared throws
propagate) or re-throw the caught IOException after logging so callers of
generateFile receive the error; locate the generateFile method (uses
createBitMatrix, MatrixToImageConfig, MatrixToImageWriter.writeToStream and
resolveImageFormat) and implement one of these fixes so the method's throws
IOException contract is honored.
---
Outside diff comments:
In `@application-engine/pom.xml`:
- Around line 416-421: Remove the stale net.glxn.qrgen:core dependency block
from the POM (the <dependency> element for groupId net.glxn.qrgen and artifactId
core) and in the QrCode.java class remove the unused import
net.glxn.qrgen.core.image.ImageType and any references to ImageType; replace
those references with the appropriate ZXing/Java types used elsewhere (e.g.,
java.awt.image.BufferedImage or ZXing's matrix-to-image utilities) so the class
compiles after the dependency is removed.
In
`@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.java`:
- Line 5: Remove the stale import of net.glxn.qrgen.core.image.ImageType from
the QrCode class: locate the import statement referencing ImageType in
QrCode.java and delete it so the class only uses the new ZXing-backed
QrImageType; ensure no other references to ImageType remain and compile to
verify the unused import is gone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ef9c6230-de06-47b2-a537-562d2e721565
📒 Files selected for processing (4)
application-engine/pom.xmlapplication-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.javaapplication-engine/src/main/java/com/netgrif/application/engine/business/qr/QrImageType.javaapplication-engine/src/main/java/com/netgrif/application/engine/business/qr/QrService.java
- Remove unused IOException logging in `QrService`. - Update `QrCode` defaults: error correction to H, off-color to white.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.java (1)
5-5:⚠️ Potential issue | 🟡 MinorRemove unused import from previous library.
The import
net.glxn.qrgen.core.image.ImageTypeis no longer used after switching toQrImageType. This is leftover from the qrgen library that was replaced.Proposed fix
import com.google.zxing.qrcode.decoder.ErrorCorrectionLevel; import lombok.Data; -import net.glxn.qrgen.core.image.ImageType; `@Data`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.java` at line 5, Remove the stale import net.glxn.qrgen.core.image.ImageType from the QrCode class; locate the import statement in the top of the QrCode.java file (related to class QrCode) and delete it so only the new QrImageType usage remains and there are no unused imports reported.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrService.java`:
- Line 158: QrService currently creates BufferedImage instances with
BufferedImage.TYPE_INT_ARGB (e.g., the variable 'scaled' and the composed image
created later), which is incompatible with JPEG output; update the image
creation logic to choose image type based on the requested output format: use
BufferedImage.TYPE_INT_RGB when imageType equals "JPG" or "JPEG", otherwise use
TYPE_INT_ARGB (or keep existing PNG behavior). Apply the same conditional
selection to all places creating BufferedImage for logo composition (including
the creation at lines around 180-184) so that ImageIO.write gets an RGB image
for JPEGs and avoid transparent/black artifacts.
---
Outside diff comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.java`:
- Line 5: Remove the stale import net.glxn.qrgen.core.image.ImageType from the
QrCode class; locate the import statement in the top of the QrCode.java file
(related to class QrCode) and delete it so only the new QrImageType usage
remains and there are no unused imports reported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d0bb546e-b791-434e-878d-28724b25cabc
📒 Files selected for processing (2)
application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrCode.javaapplication-engine/src/main/java/com/netgrif/application/engine/business/qr/QrService.java
- Remove unused IOException logging in `QrService`. - Update `QrCode` defaults: error correction to H, off-color to white.
6b0a5dd
- Add error logging for file write failures in `QrService`. - Enhance exception handling with detailed log context.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrService.java (2)
161-161:⚠️ Potential issue | 🟡 MinorAlpha channel incompatibility with JPEG output.
BufferedImage.TYPE_INT_ARGBincludes an alpha channel, but JPEG format does not support transparency. When the output format is JPEG, transparent areas may render as black or produce visual artifacts.Since the default is now PNG (which supports alpha), this only affects explicit JPEG usage. Consider documenting that PNG is recommended for logo compositing, or conditionally use
TYPE_INT_RGBwhen the target format is JPEG.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrService.java` at line 161, The new BufferedImage uses TYPE_INT_ARGB which includes alpha and will produce artifacts if the caller requests JPEG output; modify the QR image creation so that when the target format is JPEG (detect via the outputFormat/format variable or method parameter used by the QR creation routine) you instantiate the scaled BufferedImage with TYPE_INT_RGB and prefill its background (e.g., white) before drawing the QR/logo, otherwise keep TYPE_INT_ARGB for PNG; also add a short comment or log noting PNG is preferred for logos with transparency.
183-186:⚠️ Potential issue | 🟡 MinorSame alpha channel consideration applies here.
The combined image also uses
TYPE_INT_ARGB. The same JPEG compatibility note applies - this image is ultimately written viawriteImageToFile, and JPEG output may produce unexpected results with alpha channel data.As mentioned above, documenting PNG as the recommended format for logo compositing would address this concern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/master-build.yml:
- Around line 106-112: The "Generate certificates" step currently uses manual cd
hops which are brittle; update that step to set working-directory:
application-engine/src/main/resources/certificates instead of running cd, remove
the trailing cd ../../../../.. command, and keep the openssl commands (genrsa,
rsa -pubout, pkcs8) running directly in that working directory so the job
returns to the workspace root for the next step automatically.
- Line 77: Replace the dynamic expression used for ELASTIC_SEARCH_URL to the
hardcoded port to avoid the actionlint error: change the value that currently
references job.services.elasticsearch.ports[9200] to the literal URL
"http://localhost:9200" so ELASTIC_SEARCH_URL uses the statically pinned port
9200 instead of the numeric-indexed lookup.
---
Duplicate comments:
In
`@application-engine/src/main/java/com/netgrif/application/engine/business/qr/QrService.java`:
- Line 161: The new BufferedImage uses TYPE_INT_ARGB which includes alpha and
will produce artifacts if the caller requests JPEG output; modify the QR image
creation so that when the target format is JPEG (detect via the
outputFormat/format variable or method parameter used by the QR creation
routine) you instantiate the scaled BufferedImage with TYPE_INT_RGB and prefill
its background (e.g., white) before drawing the QR/logo, otherwise keep
TYPE_INT_ARGB for PNG; also add a short comment or log noting PNG is preferred
for logos with transparency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c968ae42-57b5-4ca3-a6e1-e8bd3e552041
📒 Files selected for processing (2)
.github/workflows/master-build.ymlapplication-engine/src/main/java/com/netgrif/application/engine/business/qr/QrService.java
Kovy95
left a comment
There was a problem hiding this comment.
add catch block that are in @renczesstefan comments
Description
Change
qrgenlibrary withzxingfor QR code generation.Fixes NAE-2405
Dependencies
Third party dependencies
Blocking Pull requests
There are no dependencies on other PR
How Has Been This Tested?
Test Configuration
<Please describe configuration for tests to run if applicable, like program parameters, host OS, VM configuration etc.>
Checklist:
Summary by CodeRabbit
New Features
Improvements